Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tests due to Node.js changing the JSON.parse error messages #635

Merged
merged 4 commits into from
Jul 8, 2024

Conversation

lydell
Copy link
Collaborator

@lydell lydell commented Jul 8, 2024

Fixes #633.

Closes #634.

CI failed, due to the test matrix, so updated that too. Then macOS-latest required elm-tooling and elm-json to be updated.

Comment on lines +47 to +56
(error) => {
assert.strictEqual(
error.message.replace(
// Handle slightly different JSON.parse error messages on different Node.js versions.
/^.+ in JSON at position .+$/gm,
'(the JSON parse error)'
),
expected
);
return true;
Copy link

@lishaduck lishaduck Jul 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can pass an assertion there 🤦‍♂️
Now that makes sense.

Copy link
Collaborator Author

@lydell lydell Jul 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I had to look that up the docs and play around with it in my editor. Otherwise I would have left a comment on your PR. Hope that’s ok 😄

This turned out to be difficult to merge anyway due to other things being out of date, and me trying to ambitious and not just do the simplest fix…

Copy link

@lishaduck lishaduck Jul 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I had to look that up the docs and play around with it in my editor. Otherwise I would have left a comment on your PR. Hope that’s ok 😄

That's fine 😁

This turned out to be difficult to merge anyway due to other things being out of date, and me trying to ambitious not just do the simplest fix…

I'd be the same way 😅

@lydell lydell merged commit 3c18757 into master Jul 8, 2024
20 checks passed
@lydell lydell deleted the fix-tests branch July 8, 2024 21:00
@lishaduck
Copy link

lishaduck commented Jul 8, 2024

🎉1

Footnotes

  1. For the much-felt lack of emoji reactions to merges 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mocha tests fail on modern Node versions
2 participants